fix(webview): route webview:changed only to its owner in multi-user mode - #501
Merged
Merged
Conversation
webview:changed carried only {action, id} and the SSE routing hint had no
webview: branch, so every connected client received it: in multi-user mode
any user saw the ids of other users' web-tab creates, edits and deletes.
The event now carries the web tab's owner (from the stored record) and is
routed to that owner plus admins. Single-user delivery is unchanged.
Owner
|
Merged, and it ships in 1.33.2. Thanks @aakhter. This was a real leak in multi-user mode and the fix is exactly the right size: the owner comes from the stored web-tab record rather than from whoever made the request, so an admin editing someone's tab still notifies that person, and the new |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
In multi-user mode, every connected SSE client received
webview:changed, so any user saw the ids of other users' web-tab creates, edits and deletes.webview-routes.tsbroadcast{ action, id }with no owner, and the server's SSE routing hint has branches fortab:, the session prefixes,remote:andclipboard:but none forwebview:, so the hint came back undefined and the event went to everyone.How
webview:changedbroadcasts now carryowner: ownerLayoutKey(<record>.owner). The owner comes from the stored web-tab record, not from whoever made the request, so when an admin edits or deletes a user's tab, that user is notified.src/web/webview-sse.ts(deriveWebviewSseHint, 6 lines) returns{ username: owner, sessionScoped: true }, andserver.tsrouteswebview:events through it, next to the existingtab:branch.owner: '@single', an optional field (non-breaking perdocs/versioning-policy.md).webview-tabs.jsonly readsactionandidbefore re-fetching, so it ignores it. Thesse-events.tsdoc comment lists the new field.Testing
test/webview-sse.test.ts(7) runs the real server hint function and the routes end to end. The multi-user isolation tests failed before the fix (bob received alice's create, update and delete). Removing the newwebview:branch makes 4 of them fail; the 3 single-user tests pass either way, as they should.webview-routes(27),tab-layout-sse(8) andsse-registry-parity(4).typecheck,lint,format:checkandbuildare clean.